Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toolkit: cdk ls --long #380

Merged
merged 1 commit into from
Jul 23, 2018
Merged

Toolkit: cdk ls --long #380

merged 1 commit into from
Jul 23, 2018

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Jul 19, 2018

Default "ls" to just print the names of all stacks.
The --long (or -l) switch can be used to emit all data.

Default "ls" to just print the names of all stacks.
The `--long` (or `-l`) switch can be used to emit all data.
@eladb eladb requested a review from RomainMuller July 19, 2018 15:57

// just print stack names
for (const stack of stacks) {
data(stack.name);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return stacks.map(s => s.name);?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that will cause a JSON/YAML array to be returned:

- stack1
- stack2
- stack3

And I want an "xargs-compatible" output:

stack1
stack2
stack3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. As a user I would sometimes find this convenient, sometimes find it confusing. I wonder if long shouldn't be the default, and the xargs-compatible be the option...

Anyway -- that is no blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ls should behave like "ls", where --long is not the default 😉

@eladb
Copy link
Contributor Author

eladb commented Jul 23, 2018

@RomainMuller any other feedback here?

@eladb eladb merged commit 5209555 into master Jul 23, 2018
@eladb eladb deleted the benisrae/toolkit-ls-long branch July 23, 2018 10:38
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants